feat: add support for describe command #96
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
maanugh
changed the title
pkg/cmd/describe/describe_rollout.go
Outdated
| fmt.Fprintf(o.Out, tableFormat, "Strategy:", "Canary") | ||
|
|
||
| if canary := rollout.Spec.Strategy.Canary; canary != nil { | ||
| fmt.Fprintf(o.Out, tableFormat, " Step:", canary.Steps[0].Replicas) |
There was a problem hiding this comment.
the content in these field is incorrect, and shall we display the current step or all canary steps? I think current step is required.
- step: the total number of canary steps can be get from the length of rollout.Spec.Strategy.Canary.steps, and current step can be calculated as rollout.status.canaryStatus.currentStepIndex + 1)
- SetWeight is very confusing, i prefer trafficWeight, and it can be get from the rollout.Spec.Strategy.Canary.steps[*].weight if available.
There was a problem hiding this comment.
conclusion -> we would showing each step, and highlighting the currentStepIndex .
Strategy: Canary
Step: 3/3
Steps:
- Replicas: 1
traffic: 5%
- Replicas: 50%
- Replicas: 100%
pkg/cmd/describe/describe_rollout.go
Outdated
| info.Replicas.Current = int32(status.FieldByName("Replicas").Int()) | ||
| info.Replicas.Updated = int32(status.FieldByName("UpdatedReplicas").Int()) | ||
| info.Replicas.Ready = int32(status.FieldByName("ReadyReplicas").Int()) | ||
| info.Replicas.Available = int32(status.FieldByName("AvailableReplicas").Int()) |
There was a problem hiding this comment.
plz check the rollout.status.ObservedGeneration against rollout.Metadata.generation, if two generation does not match, we shall not display status related fields.
|
Updated view - > |
pkg/cmd/describe/describe_rollout.go
Outdated
| BatchID: pod.Labels["rollouts.kruise.io/rollout-batch-id"], | ||
| Status: string(pod.Status.Phase), | ||
| Age: duration.HumanDuration(time.Since(pod.CreationTimestamp.Time)), | ||
| Restarts: fmt.Sprintf("%v (%v ago)", strconv.Itoa(int(pod.Status.ContainerStatuses[0].RestartCount)), duration.HumanDuration(time.Since(pod.Status.ContainerStatuses[0].LastTerminationState.Terminated.FinishedAt.Time))), |
There was a problem hiding this comment.
plz sum the restart count of all containers in the pod
| } | ||
|
|
||
| // Traffic Routings | ||
| if len(rollout.Spec.Strategy.Canary.TrafficRoutings) > 0 { |
There was a problem hiding this comment.
for end-to-end canary release, it is also possible for rollout to use rollout.spec.strategy.canary.trafficRoutingRef to specify traffic routing obj, plz refer to
https://openkruise.io/rollouts/user-manuals/strategy-end2end-canary-update
There was a problem hiding this comment.
if we have the ref of traffic routing then we have to fetch the detail from the api server. //TrafficRoutingRef
There was a problem hiding this comment.
there is problem in the api .
|
plz also update the user docs in |
pkg/cmd/describe/describe_rollout.go
Outdated
| BatchID: pod.Labels["rollouts.kruise.io/rollout-batch-id"], | ||
| Status: string(pod.Status.Phase), | ||
| Age: duration.HumanDuration(time.Since(pod.CreationTimestamp.Time)), | ||
| Restarts: fmt.Sprintf("%v (%v ago)", strconv.Itoa(int(pod.Status.ContainerStatuses[0].RestartCount)), duration.HumanDuration(time.Since(pod.Status.ContainerStatuses[0].LastTerminationState.Terminated.FinishedAt.Time))), |
There was a problem hiding this comment.
this line will panic :
- pod.status.containerstatus may be empty for a newly created pod
- pod.status.containerstatus[0].LastTerminationState.Terminated may also be null
There was a problem hiding this comment.
lastTerminationState unsure about what to do ?
Signed-off-by: maanugh <[email protected]>
maanugh
force-pushed
the
feat/describe
branch
from
c452f61
to
b4c4f4d
Compare
Signed-off-by: maanugh <[email protected]>
Signed-off-by: maanugh <[email protected]>
closes #94
Adding support to visualize kruise rollout using the command
kruise describe rollout rollouts-2